Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "Secure Mode" to the complete example #112

Merged
merged 26 commits into from
Apr 4, 2023

Conversation

RothAndrew
Copy link
Member

@RothAndrew RothAndrew commented Mar 24, 2023

Closes #101

@RothAndrew
Copy link
Member Author

/test all

@RothAndrew RothAndrew changed the title [WIP] Add "Secure Mode" to the complete example Add "Secure Mode" to the complete example Mar 24, 2023
@RothAndrew RothAndrew marked this pull request as ready for review March 24, 2023 22:04
@RothAndrew RothAndrew requested review from a team as code owners March 24, 2023 22:04
@RothAndrew
Copy link
Member Author

/test all

wirewc
wirewc previously approved these changes Mar 25, 2023
ntwkninja
ntwkninja previously approved these changes Mar 25, 2023
Copy link
Member

@ntwkninja ntwkninja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lg2m

@RothAndrew
Copy link
Member Author

/test all

@RothAndrew
Copy link
Member Author

Getting an error on the secure mode test:

image

But it passes locally. At first I thought it might be because of the differences between using managed and self-managed node groups. Currently testing in #116.

But now because it passes locally I'm wondering whether it is due to the difference in the type of auth being used. Locally I'm using my own AWS creds, but in the pipeline it uses an OIDC provider wired up with GitHub, so it isn't a "user" applying the resource, rather it is an assumed role doing it.

I'm wondering if implementing #105 and automatically adding the role that is currently being used will fix the issue.

What's weird is, this issue is not seen in the "insecure mode" of the complete example. I'm guessing because insecure mode uses managed nodegroups, which means we don't actually create the aws-auth configmap, amazon does

@RothAndrew
Copy link
Member Author

My test in #116 passed so now I'm thinking the issue here does not lie in the aws-auth configmap stuff. What is super weird is that it works locally but not in the pipeline. Perhaps something with the sshuttle stuff

@RothAndrew
Copy link
Member Author

Another difference that I want to check out is that locally I'm only running one of the tests

make test-complete-secure

But in the pipeline both tests are running in parallel

make test

@RothAndrew RothAndrew dismissed stale reviews from ntwkninja and wirewc via 33f4f82 March 27, 2023 18:58
@RothAndrew
Copy link
Member Author

/test all

@RothAndrew
Copy link
Member Author

/test all

@RothAndrew
Copy link
Member Author

/test all

@RothAndrew
Copy link
Member Author

/test all

@RothAndrew
Copy link
Member Author

/test all

@RothAndrew
Copy link
Member Author

/test all

@RothAndrew
Copy link
Member Author

Issue has most likely been narrowed down to a race condition with the EKS cluster. Terraform was trying to use the Kubernetes Provider / Helm Provider / Kubectl Provider to deploy stuff to the cluster before it was ready.

To resolve I switched from 2 terraform apply runs to 3 runs: (1) terraform apply -target=module.vpc -target=module.bastion (2) terraform apply -target=module.vpc -target=module.bastion -target=module.eks (via sshuttle) (3) terraform apply (via sshuttle)

I also added a 30 second wait between step 2 and step 3. Doing this gives the cluster enough time to be ready to accept deployments when Terraform tries to do them.

@RothAndrew
Copy link
Member Author

RothAndrew commented Mar 28, 2023

Currently blocked by https://github.com/defenseunicorns/iac/pull/121 Edit: merged, no longer blocked

@RothAndrew
Copy link
Member Author

/test all

1 similar comment
@RothAndrew
Copy link
Member Author

/test all

@zack-is-cool
Copy link
Member

/test all

@RothAndrew
Copy link
Member Author

/test all

@RothAndrew
Copy link
Member Author

/test all

1 similar comment
@RothAndrew
Copy link
Member Author

/test all

@RothAndrew
Copy link
Member Author

/test all

@RothAndrew
Copy link
Member Author

/test all

@RothAndrew
Copy link
Member Author

/test all

Copy link
Member

@zack-is-cool zack-is-cool left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@ntwkninja ntwkninja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

@RothAndrew RothAndrew merged commit 486b2fc into main Apr 4, 2023
@RothAndrew RothAndrew deleted the feature/secure-mode-to-complete-example branch April 4, 2023 01:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add "secure mode" to the complete example
4 participants